Feat/increase coverage test screens - #63
Merged
Merged
Conversation
Contributor
WalkthroughChangesThe PR adds shared RF dialogs, snackbars, and text fields; updates related UI and Health Connect behavior; expands CI triggers; and adds reusable Flutter test utilities with broad screen, widget, service, and user-flow coverage. UI changes
Health Connect integration
Flutter widget test coverage
CI and toolchain configuration
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@workout-logger/lib/screens/widgets/rf_dialogs.dart`:
- Around line 44-60: Update the semantic snackbar styling in the surrounding
dialog builder so success, warning, and error variants use a foreground color
meeting 4.5:1 contrast against their backgrounds for the 14px message text and
icon. Adjust the Icon and Text styling together, while preserving the existing
neutral styling and semantic colors as accents where appropriate.
In `@workout-logger/lib/screens/widgets/rf_widgets.dart`:
- Around line 945-970: Update the TextField wrapper in the RF widget to track
focus using a FocusNode or equivalent focus state, and change the outer
Container border to a high-contrast AppColors.primary border while focused.
Preserve the existing glass border for unfocused fields and keep the TextField’s
InputBorder.none configuration unchanged.
In `@workout-logger/test/screens/ai_coach_screen_test.dart`:
- Around line 12-31: Strengthen the state-specific assertions in
workout-logger/test/screens/ai_coach_screen_test.dart lines 12-31 by asserting
the missing-API-key prompt/banner in addition to the shared “AI Coach” title;
also update
workout-logger/test/screens/programs/program_designer_screen_test.dart lines
40-59 to assert its validation error message/control,
workout-logger/test/screens/widgets/targets_tab_test.dart lines 29-53 to assert
a rendered active-target item or value, and
workout-logger/test/screens/workout_flow_screen_test.dart lines 27-37 to assert
quick-start-specific content or controls.
In `@workout-logger/test/screens/edit_workout_session_screen_test.dart`:
- Around line 73-90: Update the test named “Edits session notes and saves
session” to verify persistence through storage, not only the in-memory provider.
After tapping Save, retrieve the session with await
storage.getWorkoutSession(session.id) and assert its notes equal “Updated
workout session note”; retain the existing provider assertion only if useful.
- Around line 42-56: Strengthen the edit-action tests in
workout-logger/test/screens/edit_workout_session_screen_test.dart: in the “Adds
a set to an existing exercise” test (lines 42-56), assert the target exercise’s
set count increases; in the selected-exercise removal test (lines 58-71), assert
its set count decreases; and in the discard test (lines 92-109), assert both the
discard dialog and EditWorkoutSessionScreen are dismissed after tapping Discard.
In `@workout-logger/test/screens/heart_rate_detail_screen_test.dart`:
- Line 14: Stop discarding widget-test exceptions: in
workout-logger/test/screens/heart_rate_detail_screen_test.dart:14,
history_screen_test.dart:30 and :55, home_screen_test.dart:88-93,
settings_screen_test.dart:29, and sleep_detail_screen_test.dart:14, assert each
tester.takeException() result is null. In
workout-logger/test/screens/home_screen_test.dart:66, fix the underlying
overflow/layout issue rather than clearing the exception.
- Line 16: Update the HeartRateDetailScreen test in
workout-logger/test/screens/heart_rate_detail_screen_test.dart at lines 16-16 to
assert the displayed title and granularity controls/content instead of only
finding the explicitly pumped widget. Update the history screen test in
workout-logger/test/screens/history_screen_test.dart at lines 32-32 to verify
the empty-history UI and that no session items are present.
In `@workout-logger/test/screens/onboarding_screen_test.dart`:
- Around line 23-28: Update the shared TestRobot helper methods, including
pumpScreen and the helpers for tap, fill, and pop operations, to call
tester.takeException() and assert that the result is null after each operation.
Ensure framework exceptions fail the test instead of being silently ignored.
In `@workout-logger/test/screens/widgets/health_widgets_test.dart`:
- Line 91: Update the testWidgets case around tester.takeException() to capture
its returned exception and assert that it is null, rather than discarding it.
Preserve the existing test flow while ensuring unexpected framework rendering
failures cannot be masked by the skipOnError wrapper.
In `@workout-logger/test/test_utils/test_harness.dart`:
- Around line 76-82: Remove the global FlutterError.onError override that
filters messages containing “RenderFlex” or “overflowed” from the test harness.
Update the owning test to fix or explicitly assert the expected layout failure
instead, allowing framework errors to propagate through FlutterError.onError
normally.
- Around line 24-32: Make the required widget parameters explicit in both helper
methods: update TestHarness.wrap in
workout-logger/test/test_utils/test_harness.dart at lines 24-32 to use an empty
required-parameter list followed by required Widget child, and update the
corresponding method in workout-logger/test/test_utils/test_robot.dart at lines
18-24 to use required Widget widget in the same ordering.
In `@workout-logger/test/test_utils/test_robot.dart`:
- Line 34: Replace every discarded tester.takeException() call with an assertion
that the result is null, unless that test intentionally expects a specific
exception. Apply this in workout-logger/test/test_utils/test_robot.dart lines
34-34, 43-43, 52-52, and 59-59; workout-logger/test/test_utils/test_sweep.dart
line 24-24; workout-logger/test/screens/ai_coach_screen_test.dart line 29-29;
workout-logger/test/screens/ai_program_generator_screen_test.dart lines 24-24
and 33-33; workout-logger/test/screens/profile_screen_test.dart line 33-33;
workout-logger/test/screens/programs/program_designer_screen_test.dart lines
32-32, 50-50, 55-55, 71-71, and 81-81; and
workout-logger/test/screens/workout_flow_screen_test.dart lines 21-21 and 34-34.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2cbbe829-6fb3-4ee3-8c53-9dfdb8c0563a
📒 Files selected for processing (24)
.github/workflows/test.ymlworkout-logger/lib/screens/edit_workout_session_screen.dartworkout-logger/lib/screens/widgets/floating_nav_bar.dartworkout-logger/lib/screens/widgets/rf_dialogs.dartworkout-logger/lib/screens/widgets/rf_widgets.dartworkout-logger/test/screens/ai_coach_screen_test.dartworkout-logger/test/screens/ai_program_generator_screen_test.dartworkout-logger/test/screens/edit_workout_session_screen_test.dartworkout-logger/test/screens/heart_rate_detail_screen_test.dartworkout-logger/test/screens/history_screen_test.dartworkout-logger/test/screens/home_screen_test.dartworkout-logger/test/screens/onboarding_screen_test.dartworkout-logger/test/screens/profile_screen_test.dartworkout-logger/test/screens/programs/program_designer_screen_test.dartworkout-logger/test/screens/settings_screen_test.dartworkout-logger/test/screens/sleep_detail_screen_test.dartworkout-logger/test/screens/widgets/health_widgets_test.dartworkout-logger/test/screens/widgets/rf_cards_test.dartworkout-logger/test/screens/widgets/targets_tab_test.dartworkout-logger/test/screens/workout_flow_screen_test.dartworkout-logger/test/test_utils/test_fixtures.dartworkout-logger/test/test_utils/test_harness.dartworkout-logger/test/test_utils/test_robot.dartworkout-logger/test/test_utils/test_sweep.dart
Comment on lines
+23
to
+28
| await robot.pumpScreen( | ||
| WelcomePage(onComplete: () => completed = true), | ||
| storage: storage, | ||
| settingsProvider: settings, | ||
| workoutProvider: workout, | ||
| ); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make TestRobot fail on framework exceptions.
TestRobot.pumpScreen calls tester.takeException() without asserting it is null, so this test can silently pass after a rendering failure. Change the shared helper to assert no exception after pump, tap, fill, and pop operations.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/test/screens/onboarding_screen_test.dart` around lines 23 -
28, Update the shared TestRobot helper methods, including pumpScreen and the
helpers for tap, fill, and pop operations, to call tester.takeException() and
assert that the result is null after each operation. Ensure framework exceptions
fail the test instead of being silently ignored.
Comment on lines
+24
to
+32
| static Widget wrap( | ||
| Widget child, { | ||
| MockStorageService? storage, | ||
| WorkoutProvider? workoutProvider, | ||
| SettingsProvider? settingsProvider, | ||
| HistoryManager? historyManager, | ||
| HealthHistoryManager? healthHistoryManager, | ||
| Size viewportSize = const Size(1080, 2400), | ||
| }) { |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo files of interest:"
git ls-files | rg '(^|/)(test_harness|test_robot)\.dart$|analysis_options\.ya?ml' || true
echo
echo "test_harness outline:"
ast-grep outline workout-logger/test/test_utils/test_harness.dart --view expanded || true
echo
echo "test_robot outline:"
ast-grep outline workout-logger/test/test_utils/test_robot.dart --view expanded || true
echo
echo "Relevant snippets:"
cat -n workout-logger/test/test_utils/test_harness.dart | sed -n '1,120p'
echo
cat -n workout-logger/test/test_utils/test_robot.dart | sed -n '1,140p'
echo
echo "Call sites named args:"
rg -n "\bwrap\(|testRobot\(|pumpScreen\(" workout-logger/test -SRepository: Devasy/RepForge
Length of output: 12149
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "analysis_options.yaml:"
cat -n workout-logger/analysis_options.yaml | sed -n '1,160p'
echo
echo "Full call-site snippets with formatting context:"
for f in \
workout-logger/test/screens/settings_screen_test.dart \
workout-logger/test/screens/profile_screen_test.dart \
workout-logger/test/screens/ai_program_generator_screen_test.dart \
workout-logger/test/screens/workout_flow_screen_test.dart \
workout-logger/test/screens/onboarding_screen_test.dart \
workout-logger/test/screens/widgets/targets_tab_test.dart \
workout-logger/test/screens/rf_cards_test.dart \
workout-logger/test/screens/history_screen_test.dart \
workout-logger/test/screens/widgets/health_widgets_test.dart \
workout-logger/test/screens/widgets/health_widgets_test.dart
do
if [ -f "$f" ]; then
echo "--- $f ---"
cat -n "$f" | sed -n '1,90p'
fi
done
echo
echo "Precise call formats for TestHarness.wrap and pumpScreen:"
python3 - <<'PY'
from pathlib import Path
import re
def read(f):
return Path(f).read_text()
for path, pattern in [
("workout-logger/test", r"\bTestHarness\.wrap\s*\("),
("workout-logger/test/test_utils/test_robot.dart", r"\bpumpScreen\s*\("),
]:
files = list(Path("workout-logger/test").glob("**/*.dart"))
for f in files:
text = f.read_text(errors="replace")
if re.search(pattern, text):
lines = text.splitlines()
for i, line in enumerate(lines, 1):
if re.search(pattern, line):
start = max(1, i-1)
end = min(len(lines), i+8)
block = "\n".join(f"{j}: {lines[j-1]}" for j in range(start, end+1))
print(path+f.name, f"line {i}", block, sep="\n-")
PYRepository: Devasy/RepForge
Length of output: 34969
🌐 Web query:
Dart style guide use named required for required named parameters
💡 Result:
In Dart, named parameters are optional by default [1][2][3]. To make a named parameter mandatory, you must explicitly use the 'required' keyword in the parameter declaration [1][2][4]. There is no specific rule in the official Effective Dart style guide that mandates the use of 'required' for all named parameters; rather, it is a functional language feature used when a caller must provide a value [1][2]. However, the Dart ecosystem and linter enforce specific stylistic conventions regarding their placement: 1. Mandatory Usage: You use the 'required' modifier when you want to ensure that callers explicitly provide a value for a named parameter [1][4]. This is often used for non-nullable types that do not have a default value, as it satisfies sound null safety requirements without forcing the parameter to be nullable [5][4]. 2. Placement Convention: The Dart linter includes a rule (always_put_required_named_parameters_first) that recommends placing all 'required' named parameters before any optional named parameters in the parameter list [6]. This improves readability and API clarity [6]. Example of correct usage and ordering: void exampleFunction({required int requiredParam, int? optionalParam}) { // Function body } While 'required' is a necessary tool for defining mandatory named arguments, its usage should be dictated by your API design requirements—specifically, whether a parameter is essential for the function's execution—rather than a blanket style guide mandate [1][4].
Citations:
- 1: https://dart.dev/language/functions
- 2: https://github.com/dart-lang/site-www/blob/main/src/content/language/functions.md
- 3: https://github.com/dart-lang/language/blob/main/working/simpler-parameters/feature-specification.md
- 4: https://docs.syntblaze.com/dart/parameters/required-named-parameter
- 5: https://dart.dev/tools/diagnostics/missing_default_value_for_parameter
- 6: https://dart.googlesource.com/sdk/+/40323dfaafee4fe1b1008d85155232da462f01c9/pkg/linter/messages.yaml
🌐 Web query:
flutter_lints flutter.yaml rules prefer_named_parameters required named parameters
💡 Result:
There is no official lint rule named prefer_named_parameters in the standard Dart SDK or the flutter_lints package [1][2][3]. The flutter_lints package provides a curated, recommended set of lints for Flutter projects but does not include rules that enforce the use of named parameters over positional ones by default [1][4][5]. However, there are official Dart lint rules related to named parameters that you can enable manually in your analysis_options.yaml file [2][3]: 1. always_put_required_named_parameters_first: This rule enforces that required named parameters must appear before optional named parameters in a function definition [6][7]. 2. avoid_positional_boolean_parameters: While not strictly a "prefer named parameters" rule, this encourages better API design by warning against positional boolean arguments, which are often ambiguous, and suggesting named parameters instead [8]. If you specifically require a rule that warns against using positional parameters when named parameters would be more appropriate (e.g., for functions with many parameters), this is considered an opinionated style choice [9]. Such rules are generally not included in standard, community-wide lint packages like flutter_lints [1][10][9]. If you wish to use such a rule, you would need to look into third-party packages (e.g., saropa_lints) or implement a custom lint rule [10][9]. To enable any of the standard rules mentioned above, add them to your analysis_options.yaml file: linter: rules: - always_put_required_named_parameters_first - avoid_positional_boolean_parameters
Citations:
- 1: https://pub.dev/packages/flutter_lints
- 2: https://dart.dev/tools/linter-rules
- 3: https://dart.dev/tools/linter-rules/all
- 4: https://docs.flutter.dev/release/breaking-changes/flutter-lints-package
- 5: https://github.com/flutter/packages/tree/main/packages/flutter_lints
- 6: https://dart.dev/tools/linter-rules/always_put_required_named_parameters_first
- 7: https://dart.dev/tools/diagnostics/always_put_required_named_parameters_first
- 8: https://dart.dev/tools/linter-rules/avoid_positional_boolean_parameters
- 9: https://pub.dev/documentation/saropa_lints/latest/saropa_lints/PreferAllNamedParametersRule-class.html
- 10: https://pub.dev/documentation/saropa_lints/latest/saropa_lints/PreferNamedParametersRule-class.html
Make the required test-wrapping widgets explicit.
These helpers always need a widget, so mark it in the parameter ordering suggested by the flutter_lints config:
workout-logger/test/test_utils/test_harness.dart#L24-L26: userequired Widget childafter the empty required parameter list.workout-logger/test/test_utils/test_robot.dart#L18-L20: userequired Widget widgetafter the empty required parameter list.
📍 Affects 2 files
workout-logger/test/test_utils/test_harness.dart#L24-L32(this comment)workout-logger/test/test_utils/test_robot.dart#L18-L24
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/test/test_utils/test_harness.dart` around lines 24 - 32, Make
the required widget parameters explicit in both helper methods: update
TestHarness.wrap in workout-logger/test/test_utils/test_harness.dart at lines
24-32 to use an empty required-parameter list followed by required Widget child,
and update the corresponding method in
workout-logger/test/test_utils/test_robot.dart at lines 18-24 to use required
Widget widget in the same ordering.
Source: Coding guidelines
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## r2.1.0 #63 +/- ##
===========================================
+ Coverage 48.76% 74.90% +26.14%
===========================================
Files 87 88 +1
Lines 14432 14491 +59
===========================================
+ Hits 7038 10855 +3817
+ Misses 7394 3636 -3758 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Owner
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
workout-logger/lib/services/health_connect_service.dart (1)
334-349: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake the sync path idempotent around timed-out writes.
Future.timeoutdrops the result but does not stopwriteRecords; Health Connect may still accept the workout before the timeout fires. If this catch returnsfalseand callers retry without a stableclientRecordIdin the record metadata, a late completion can create a duplicate workout. Add idempotent metadata for the session and avoid treating a write timeout/readback failure as a non-synced workout.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/services/health_connect_service.dart` around lines 334 - 349, Update the session write flow around writeRecords and the subsequent readback so each workout record includes a stable clientRecordId derived from the session identity, allowing retries to be idempotent. Treat write timeouts and readback failures as an unknown or potentially synced outcome rather than returning false, and remove the temporary debug readback logging.workout-logger/test/test_utils/test_sweep.dart (1)
8-19: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMake
finderimmutable.Every reachable branch assigns it once; declare it
final.Proposed fix
- Finder finder; + final Finder finder;As per coding guidelines, “Prefer
finalfor local variables in Dart code.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/test/test_utils/test_sweep.dart` around lines 8 - 19, Update the local finder variable in tapAll to be declared final, preserving the existing branch assignments and unsupported-target continue behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@workout-logger/lib/screens/widgets/routine_creator.dart`:
- Around line 162-170: Update the onReorderItem callback to allow next ==
_selectedIds.length while still rejecting larger destinations; keep the
old-index validation unchanged. Add a reorder test covering moving an exercise
into that final position before the Add Exercises slot.
In `@workout-logger/lib/services/health_connect_service.dart`:
- Around line 61-65: Replace the universal 100 ms timeout in _getConnector and
the related Health Connect operations with configurable, operation-specific
deadlines: use realistic limits for bounded calls, remove the deadline from
consent/permission requests, and allow the 5,000-record heart-rate query
sufficient time to complete. Apply this across
workout-logger/lib/services/health_connect_service.dart at lines 61-65, 76-78,
90-95, 106-110, 128-135, 149-154, 171-178, 239-246, 261-268, and 286-296,
preserving existing unavailable, denied, and empty-result handling. Add
delayed-success tests covering platform responses that exceed 100 ms but remain
within the configured operation deadline.
In `@workout-logger/test/screens/history_screen_test.dart`:
- Around line 44-48: Update the test setup around TestHarness.prepareTester to
define and pass the intended viewportSize instead of allowing the harness
default to override the configured 800×1800 viewport. Pass viewportSize to both
TestHarness.prepareTester and TestHarness.wrap, preserving the existing viewport
cleanup.
In `@workout-logger/test/screens/programs/programs_screens_test.dart`:
- Around line 25-31: Make both workflows mandatory in
programs_screens_test.dart: at lines 25-31, assert the expected empty/list UI
and replace the optional FloatingActionButton interaction with a required
interaction; at lines 69-77, require the JSON field and Validate button, tap
Validate, and assert the successful validation feedback.
In `@workout-logger/test/screens/widgets/health_bar_chart_test.dart`:
- Line 47: Replace each unasserted tester.takeException() call with an assertion
that the captured exception is null. Apply this at
workout-logger/test/screens/widgets/health_bar_chart_test.dart lines 47, 54, 97,
and 104; workout-logger/test/screens/widgets/health_cards_test.dart lines 88,
131, 138, 168, and 175; and
workout-logger/test/screens/widgets/workout_hr_section_test.dart line 76,
preserving the existing render and interaction test flows.
In `@workout-logger/test/screens/widgets/rf_dialogs_test.dart`:
- Around line 53-85: Add coverage alongside showRFConfirmDialog for the default
non-danger styling path and cancellation behavior: open a dialog without
isDanger, dismiss it via the cancel action, and assert the awaited result is
false. Keep the existing danger-confirmation assertions intact.
- Around line 40-50: Update the snackbar interaction test around the Warning,
Error, and Info taps to wait for each snackbar hide/show animation to settle
before asserting the next snackbar. Replace the single-frame pumps with the test
framework’s settling wait, preserving the existing tap order and snackbar text
assertions.
In `@workout-logger/test/screens/widgets/rf_widgets_test.dart`:
- Around line 146-175: The RFTextField test should verify its focus-dependent
border colors, not only text entry. In the test covering RFTextField, assert the
rendered border uses AppColors.glassBorder before interaction, tap the
TextField, then assert it uses AppColors.primary while focused; retain the
existing controller wiring assertions.
In `@workout-logger/test/services/health_connect_service_test.dart`:
- Around line 65-120: The tests around HealthConnectService.syncWorkoutSession
currently stop at connector initialization and never validate record
construction. Replace the failing connector setup with a successful
connector/platform fake, then assert the emitted record for each scenario:
zero-rep and custom-exercise handling, fallback spacing for identical
timestamps, and empty-session behavior. Keep the existing session inputs and
verify the records passed through the connector rather than only asserting a
false return value.
---
Outside diff comments:
In `@workout-logger/lib/services/health_connect_service.dart`:
- Around line 334-349: Update the session write flow around writeRecords and the
subsequent readback so each workout record includes a stable clientRecordId
derived from the session identity, allowing retries to be idempotent. Treat
write timeouts and readback failures as an unknown or potentially synced outcome
rather than returning false, and remove the temporary debug readback logging.
In `@workout-logger/test/test_utils/test_sweep.dart`:
- Around line 8-19: Update the local finder variable in tapAll to be declared
final, preserving the existing branch assignments and unsupported-target
continue behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: beeab699-bc57-4dac-a8e4-f077d6552f82
📒 Files selected for processing (32)
workout-logger/lib/screens/programs/import_program_screen.dartworkout-logger/lib/screens/widgets/health_detail_shell.dartworkout-logger/lib/screens/widgets/rf_dialogs.dartworkout-logger/lib/screens/widgets/rf_widgets.dartworkout-logger/lib/screens/widgets/routine_creator.dartworkout-logger/lib/services/health_connect_service.dartworkout-logger/test/screens/ai_coach_screen_test.dartworkout-logger/test/screens/ai_program_generator_screen_test.dartworkout-logger/test/screens/edit_workout_session_screen_test.dartworkout-logger/test/screens/heart_rate_detail_screen_test.dartworkout-logger/test/screens/history_screen_test.dartworkout-logger/test/screens/home_screen_test.dartworkout-logger/test/screens/onboarding_screen_test.dartworkout-logger/test/screens/profile_screen_test.dartworkout-logger/test/screens/programs/program_designer_screen_test.dartworkout-logger/test/screens/programs/programs_screens_test.dartworkout-logger/test/screens/settings_screen_test.dartworkout-logger/test/screens/sleep_detail_screen_test.dartworkout-logger/test/screens/widgets/health_bar_chart_test.dartworkout-logger/test/screens/widgets/health_cards_test.dartworkout-logger/test/screens/widgets/health_widgets_test.dartworkout-logger/test/screens/widgets/rf_dialogs_test.dartworkout-logger/test/screens/widgets/rf_widgets_test.dartworkout-logger/test/screens/widgets/targets_tab_test.dartworkout-logger/test/screens/widgets/workout_hr_section_test.dartworkout-logger/test/screens/workout_flow_screen_test.dartworkout-logger/test/services/health_connect_service_test.dartworkout-logger/test/sleep_hr_builder_test.dartworkout-logger/test/test_utils/test_harness.dartworkout-logger/test/test_utils/test_robot.dartworkout-logger/test/test_utils/test_sweep.dartworkout-logger/test/userflow_routine_creation_test.dart
💤 Files with no reviewable changes (2)
- workout-logger/lib/screens/programs/import_program_screen.dart
- workout-logger/test/sleep_hr_builder_test.dart
Comment on lines
+65
to
+120
| testWidgets('HealthConnectService handles sessions with zero reps and custom exercises', (WidgetTester tester) async { | ||
| final service = HealthConnectService(); | ||
| final session = WorkoutSession( | ||
| id: 'sess_custom', | ||
| date: DateTime.now(), | ||
| duration: 30, | ||
| notes: 'Custom notes', | ||
| exercises: [ | ||
| ExerciseLog( | ||
| exerciseId: 'custom_exercise_999', | ||
| sets: [ | ||
| WorkoutSet(weight: 0.0, reps: 0, timestamp: DateTime.now()), | ||
| WorkoutSet(weight: 50.0, reps: 10, timestamp: DateTime.now().add(const Duration(minutes: 5))), | ||
| ], | ||
| ), | ||
| ], | ||
| ); | ||
|
|
||
| final success = await service.syncWorkoutSession(session); | ||
| expect(success, isFalse); | ||
| }); | ||
|
|
||
| testWidgets('HealthConnectService handles sessions with identical timestamps (fallback spacing)', (WidgetTester tester) async { | ||
| final service = HealthConnectService(); | ||
| final now = DateTime.now(); | ||
| final session = WorkoutSession( | ||
| id: 'sess_identical_ts', | ||
| date: now, | ||
| duration: 45, | ||
| exercises: [ | ||
| ExerciseLog( | ||
| exerciseId: 'bench_press', | ||
| sets: [ | ||
| WorkoutSet(weight: 60.0, reps: 10, timestamp: now), | ||
| WorkoutSet(weight: 70.0, reps: 8, timestamp: now), | ||
| ], | ||
| ), | ||
| ], | ||
| ); | ||
|
|
||
| final success = await service.syncWorkoutSession(session, title: ''); | ||
| expect(success, isFalse); | ||
| }); | ||
|
|
||
| testWidgets('HealthConnectService handles empty sessions without exercises', (WidgetTester tester) async { | ||
| final service = HealthConnectService(); | ||
| final session = WorkoutSession( | ||
| id: 'sess_empty', | ||
| date: DateTime.now(), | ||
| duration: 20, | ||
| exercises: [], | ||
| ); | ||
|
|
||
| final success = await service.syncWorkoutSession(session); | ||
| expect(success, isFalse); | ||
| }); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift
These tests never exercise the session-segment cases named.
The mock makes _getConnector() fail, and syncWorkoutSession returns before record construction at workout-logger/lib/services/health_connect_service.dart Lines 316-317. Use a successful connector/platform fake and assert the emitted record for zero reps, custom exercises, identical timestamps, and empty sessions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/test/services/health_connect_service_test.dart` around lines
65 - 120, The tests around HealthConnectService.syncWorkoutSession currently
stop at connector initialization and never validate record construction. Replace
the failing connector setup with a successful connector/platform fake, then
assert the emitted record for each scenario: zero-rep and custom-exercise
handling, fallback spacing for identical timestamps, and empty-session behavior.
Keep the existing session inputs and verify the records passed through the
connector rather than only asserting a false return value.
Owner
Author
|
@coderabbiai review |
Owner
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
workout-logger/test/test_utils/test_harness.dart (1)
82-85: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMake teardown wait for surface reset completion.
setSurfaceSizeupdates the tester’s surface metrics and drains microtasks; calling it without awaiting in teardown can let the final metrics flush run after the test starts.Proposed fix
- addTearDown(() { + addTearDown(() async { tester.view.resetPhysicalSize(); tester.view.resetDevicePixelRatio(); - tester.binding.setSurfaceSize(null); + await tester.binding.setSurfaceSize(null); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/test/test_utils/test_harness.dart` around lines 82 - 85, Update the teardown callback containing tester.view resets and tester.binding.setSurfaceSize(null) to be asynchronous, and await setSurfaceSize(null) so surface metric updates and queued microtasks complete before teardown finishes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@workout-logger/test/screens/ai_coach_screen_full_test.dart`:
- Line 42: Update the test locals in the relevant sections of the full-screen AI
coach test, including robot and sendIcon, to use final declarations because they
are never reassigned; preserve their existing initialization and usage.
- Around line 59-78: The configured-state test around AiCoachScreen must be
deterministic: keep it offline, assert the seed prompt and suggestion UI
explicitly, and require the send control with findsOneWidget instead of
conditionally tapping it. Remove submission behavior from this render test;
cover message submission separately using a deterministic fake AI service and
await the asynchronous stream before asserting results or errors.
In `@workout-logger/test/screens/widgets/editable_exercise_card_full_test.dart`:
- Around line 63-68: Strengthen the affected tests so missing widgets cannot be
skipped: in
workout-logger/test/screens/widgets/editable_exercise_card_full_test.dart#L63-L68,
assert the “+ Add Set” finder is findsOneWidget before tapping; in
workout-logger/test/screens/workout_flow_screen_full_test.dart#L34-L119, add
assertions that routine exercises render, dropset UI toggles, and
deload-adjusted sets/intensity are shown; in
workout-logger/test/screens/profile_screen_full_test.dart#L32-L58, assert the
“kg” and “lbs” chips exist and verify unit state changes after tapping; in
workout-logger/test/userflow_program_design_and_generator_test.dart#L109-L136,
assert the prompt chip and “Generate Program” button exist and verify either the
expected error state or generated program.
In `@workout-logger/test/screens/widgets/health_cards_test.dart`:
- Around line 85-87: Update the affected widget tests around the ReadinessCard
assertions to verify visible card output rather than merely finding the root
widget type. Assert the ReadinessCard semantics label “Readiness 85 out of 100”
and confirm the expected sheet opens after each tap; apply the same
rendered-output checks to the corresponding assertions at the other referenced
test sections.
In `@workout-logger/test/screens/widgets/routine_creator_test.dart`:
- Around line 26-36: Complete the routine creation test after the optional
CheckboxListTile selection: seed an exercise deterministically, confirm the
picker with “Add 1,” tap “Save,” and assert that the routine provider contains
“Upper Body Push.” Preserve the existing routine-name entry and
exercise-selection steps.
In `@workout-logger/test/services/health_connect_service_test.dart`:
- Around line 140-156: Update the delayed-response test for
HealthConnectService.isAvailable to return a Pigeon-encoded
HealthPlatformStatus.available after the 200 ms delay, then assert the result is
true. Add or retain a separate unavailable-response test that returns the
unavailable status and asserts false, so the delayed test specifically covers a
slow successful platform response.
In `@workout-logger/test/userflow_program_design_and_generator_test.dart`:
- Around line 32-107: Update
workout-logger/test/userflow_program_design_and_generator_test.dart#L32-L107 so
the flow reads the program created by the Save Program interaction from
programManager or storage and opens/asserts ProgramDetailScreen with that
result, removing the manually constructed sampleProgram path; update
workout-logger/test/userflow_screens_sweep_test.dart#L166-L212 to tap the
finish/complete action on WorkoutFlowScreen and assert against the resulting
session, rather than pumping WorkoutSummaryScreen with a manually constructed
WorkoutSession.
In `@workout-logger/test/userflow_programs_screen_deep_test.dart`:
- Around line 91-98: Update the FAB sweep in the test to capture the initial
FloatingActionButton finder results or count before tapping, then iterate over
that stable collection while ensuring each tap still targets the intended
ProgramsScreen FABs rather than widgets introduced by navigation. Keep the
existing tap and pumpAndSettle behavior.
In `@workout-logger/test/userflow_routine_creation_test.dart`:
- Around line 155-166: Update the test CreateRoutineScreen supports reordering
exercise into final position before Add Exercises to seed at least two selected
exercises in the providers or test state before building the screen. Invoke
onReorderItem with a destination index representing the final position, then
assert the displayed exercise order changes; retain the no-exception assertion
so the test exercises the actual final-position reorder path.
In `@workout-logger/test/userflow_targets_and_muscle_sheets_full_test.dart`:
- Around line 96-103: Strengthen the workflow tests so advertised outcomes are
required rather than skipped: in
workout-logger/test/userflow_targets_and_muscle_sheets_full_test.dart:96-103,
require the FloatingActionButton and assert the add-target dialog appears; at
:138-154, require the target field and action buttons, then assert phase/week
creation and save feedback or persistence. In
workout-logger/test/screens/programs/programs_screens_test.dart:25-29, assert
the FAB reaches its expected destination, and at :73-76, assert successful
validation feedback after submitting valid JSON.
In `@workout-logger/test/userflow_targets_and_muscle_sheets_test.dart`:
- Around line 120-127: Update the ReadinessCard test setup to inject a readiness
manager containing a ready snapshot, then assert the card’s visible content and
drill-down interaction before checking for exceptions. Use the existing
harness/provider symbols and preserve the no-exception assertion after
validating the rendered state.
- Around line 59-66: Strengthen the tests around TargetsTab: after persisting
populated targets, assert the rendered target-card content rather than only
TargetsTab visibility; in the editable-card test, interact with the card’s edit
control and verify the captured callback arguments. Update the relevant test
cases and their existing callback capture setup, preserving the current
persistence and screen setup.
---
Outside diff comments:
In `@workout-logger/test/test_utils/test_harness.dart`:
- Around line 82-85: Update the teardown callback containing tester.view resets
and tester.binding.setSurfaceSize(null) to be asynchronous, and await
setSurfaceSize(null) so surface metric updates and queued microtasks complete
before teardown finishes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2aa89bc6-031a-4767-9657-ff513dff0177
⛔ Files ignored due to path filters (1)
workout-logger/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
workout-logger/lib/screens/widgets/readiness_card.dartworkout-logger/lib/screens/widgets/routine_creator.dartworkout-logger/lib/services/health_connect_service.dartworkout-logger/pubspec.yamlworkout-logger/test/screens/ai_coach_screen_full_test.dartworkout-logger/test/screens/history_screen_test.dartworkout-logger/test/screens/profile_screen_full_test.dartworkout-logger/test/screens/programs/programs_screens_test.dartworkout-logger/test/screens/widgets/editable_exercise_card_full_test.dartworkout-logger/test/screens/widgets/health_bar_chart_test.dartworkout-logger/test/screens/widgets/health_cards_test.dartworkout-logger/test/screens/widgets/rf_dialogs_test.dartworkout-logger/test/screens/widgets/rf_widgets_test.dartworkout-logger/test/screens/widgets/routine_creator_test.dartworkout-logger/test/screens/widgets/workout_hr_section_test.dartworkout-logger/test/screens/workout_flow_screen_full_test.dartworkout-logger/test/services/health_connect_service_test.dartworkout-logger/test/test_utils/test_harness.dartworkout-logger/test/test_utils/test_robot.dartworkout-logger/test/test_utils/test_sweep.dartworkout-logger/test/userflow_ai_coach_and_gemini_service_test.dartworkout-logger/test/userflow_health_and_profile_screen_test.dartworkout-logger/test/userflow_program_design_and_generator_test.dartworkout-logger/test/userflow_programs_screen_deep_test.dartworkout-logger/test/userflow_routine_creation_test.dartworkout-logger/test/userflow_screens_sweep_test.dartworkout-logger/test/userflow_services_and_ai_sweep_test.dartworkout-logger/test/userflow_targets_and_muscle_sheets_full_test.dartworkout-logger/test/userflow_targets_and_muscle_sheets_test.dart
Comment on lines
+59
to
+78
| aiService.init('valid_mock_api_key'); | ||
|
|
||
| await robot.pumpScreen( | ||
| const AiCoachScreen(seedPrompt: 'How can I improve my Bench Press?'), | ||
| storage: storage, | ||
| workoutProvider: workoutProvider, | ||
| geminiAiService: aiService, | ||
| settingsProvider: settingsProvider, | ||
| ); | ||
|
|
||
| robot.expectVisible(AiCoachScreen); | ||
| expect(find.byType(TextField), findsOneWidget); | ||
|
|
||
| final sendIcon = find.byIcon(Icons.arrow_upward_rounded); | ||
| if (sendIcon.evaluate().isNotEmpty) { | ||
| await tester.tap(sendIcon); | ||
| await tester.pump(); | ||
| } | ||
|
|
||
| expect(tester.takeException(), isNull); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Make this configured-state test deterministic and assert the behavior it names.
This uses concrete GeminiAiService with a fake key, then conditionally starts message submission. The single pump() does not await the streamed reply/error, and a missing send icon makes the test pass without the intended interaction. Keep this render test offline and assert the seed/suggestions explicitly; cover submission separately with a deterministic fake service and a required findsOneWidget send control.
Based on supplied context, workout-logger/lib/viewmodels/ai_coach_view_model.dart:77-124 streams through the supplied AI service asynchronously.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/test/screens/ai_coach_screen_full_test.dart` around lines 59 -
78, The configured-state test around AiCoachScreen must be deterministic: keep
it offline, assert the seed prompt and suggestion UI explicitly, and require the
send control with findsOneWidget instead of conditionally tapping it. Remove
submission behavior from this render test; cover message submission separately
using a deterministic fake AI service and await the asynchronous stream before
asserting results or errors.
Comment on lines
+63
to
+68
| // Tap + Add Set | ||
| final addSetBtn = find.text('+ Add Set'); | ||
| if (addSetBtn.evaluate().isNotEmpty) { | ||
| await tester.tap(addSetBtn); | ||
| expect(setAdded, isTrue); | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Conditional evaluate().isNotEmpty guards let regressions pass silently across several new test files. In each site below, an interaction (and sometimes the only functional assertion) only runs if a Finder happens to match; if the targeted widget fails to render, the guard is skipped and the test still passes, undermining the coverage this PR intends to add.
workout-logger/test/screens/widgets/editable_exercise_card_full_test.dart#L63-L68: assertfind.text('+ Add Set')isfindsOneWidgetunconditionally before tapping, so a missing button fails the test instead of skippingexpect(setAdded, isTrue).workout-logger/test/screens/workout_flow_screen_full_test.dart#L34-L119: add concrete assertions matching each test's stated intent (routine exercises rendered, dropset UI toggled, deload-adjusted sets/intensity shown) instead of relying on visibility + no-exception checks alone.workout-logger/test/screens/profile_screen_full_test.dart#L32-L58: assert the 'kg'/'lbs' chips exist unconditionally and verify the resulting unit-selection state changes after tapping, rather than only checking for absence of exceptions.workout-logger/test/userflow_program_design_and_generator_test.dart#L109-L136: assert the prompt chip and "Generate Program" button exist unconditionally, and check a concrete outcome (error state or generated program) instead of onlytester.takeException().
📍 Affects 4 files
workout-logger/test/screens/widgets/editable_exercise_card_full_test.dart#L63-L68(this comment)workout-logger/test/screens/workout_flow_screen_full_test.dart#L34-L119workout-logger/test/screens/profile_screen_full_test.dart#L32-L58workout-logger/test/userflow_program_design_and_generator_test.dart#L109-L136
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/test/screens/widgets/editable_exercise_card_full_test.dart`
around lines 63 - 68, Strengthen the affected tests so missing widgets cannot be
skipped: in
workout-logger/test/screens/widgets/editable_exercise_card_full_test.dart#L63-L68,
assert the “+ Add Set” finder is findsOneWidget before tapping; in
workout-logger/test/screens/workout_flow_screen_full_test.dart#L34-L119, add
assertions that routine exercises render, dropset UI toggles, and
deload-adjusted sets/intensity are shown; in
workout-logger/test/screens/profile_screen_full_test.dart#L32-L58, assert the
“kg” and “lbs” chips exist and verify unit state changes after tapping; in
workout-logger/test/userflow_program_design_and_generator_test.dart#L109-L136,
assert the prompt chip and “Generate Program” button exist and verify either the
expected error state or generated program.
Comment on lines
+85
to
+87
| expect(tester.takeException(), isNull); | ||
|
|
||
| expect(find.byType(ReadinessCard), findsOneWidget); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Assert visible card output, not the root widget instance.
find.byType(...) always finds the widget supplied to pumpWidget, even if its build returns SizedBox.shrink(). Assert rendered metrics/semantics and the sheet opened after each tap; for ReadinessCard, its Readiness 85 out of 100 semantics label is a suitable assertion.
Also applies to: 128-135, 165-172
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/test/screens/widgets/health_cards_test.dart` around lines 85 -
87, Update the affected widget tests around the ReadinessCard assertions to
verify visible card output rather than merely finding the root widget type.
Assert the ReadinessCard semantics label “Readiness 85 out of 100” and confirm
the expected sheet opens after each tap; apply the same rendered-output checks
to the corresponding assertions at the other referenced test sections.
Comment on lines
+26
to
+36
| // Enter routine name via RFTextField | ||
| await robot.fill(find.byType(TextField).first, 'Upper Body Push'); | ||
|
|
||
| // Tap Add Exercises button | ||
| await robot.tap('Add Exercises'); | ||
|
|
||
| // Select exercise in sheet | ||
| final checks = find.byType(CheckboxListTile); | ||
| if (checks.evaluate().isNotEmpty) { | ||
| await robot.tap(checks.first); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Complete the create-and-save flow.
This test stops after an optional checkbox tap; it never confirms the picker, saves, or asserts that a routine was created. Seed an exercise deterministically, tap Add 1 and Save, then verify the provider contains Upper Body Push.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/test/screens/widgets/routine_creator_test.dart` around lines
26 - 36, Complete the routine creation test after the optional CheckboxListTile
selection: seed an exercise deterministically, confirm the picker with “Add 1,”
tap “Save,” and assert that the routine provider contains “Upper Body Push.”
Preserve the existing routine-name entry and exercise-selection steps.
Comment on lines
+91
to
+98
| // Tap FABs | ||
| final fabs = find.byType(FloatingActionButton); | ||
| expect(fabs, findsWidgets); | ||
|
|
||
| for (int i = 0; i < fabs.evaluate().length; i++) { | ||
| await tester.tap(fabs.at(i)); | ||
| await tester.pumpAndSettle(); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
FAB sweep re-evaluates against a possibly-navigated screen.
Both fabs.at(i) and the loop bound fabs.evaluate().length are recomputed live each iteration. If a tapped FAB pushes a new route, later iterations tap FABs belonging to the new screen using stale indices instead of continuing the intended sweep of ProgramsScreen, making coverage nondeterministic across runs.
🧪 Proposed fix
- final fabs = find.byType(FloatingActionButton);
- expect(fabs, findsWidgets);
-
- for (int i = 0; i < fabs.evaluate().length; i++) {
- await tester.tap(fabs.at(i));
- await tester.pumpAndSettle();
- }
+ final fabs = find.byType(FloatingActionButton);
+ final fabCount = fabs.evaluate().length;
+ expect(fabCount, greaterThan(0));
+
+ for (int i = 0; i < fabCount; i++) {
+ await tester.tap(find.byType(FloatingActionButton).at(i));
+ await tester.pumpAndSettle();
+ // Return to ProgramsScreen if the tap navigated away.
+ if (find.byType(ProgramsScreen).evaluate().isEmpty) {
+ await tester.pageBack();
+ await tester.pumpAndSettle();
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Tap FABs | |
| final fabs = find.byType(FloatingActionButton); | |
| expect(fabs, findsWidgets); | |
| for (int i = 0; i < fabs.evaluate().length; i++) { | |
| await tester.tap(fabs.at(i)); | |
| await tester.pumpAndSettle(); | |
| } | |
| // Tap FABs | |
| final fabs = find.byType(FloatingActionButton); | |
| final fabCount = fabs.evaluate().length; | |
| expect(fabCount, greaterThan(0)); | |
| for (int i = 0; i < fabCount; i++) { | |
| await tester.tap(find.byType(FloatingActionButton).at(i)); | |
| await tester.pumpAndSettle(); | |
| // Return to ProgramsScreen if the tap navigated away. | |
| if (find.byType(ProgramsScreen).evaluate().isEmpty) { | |
| await tester.pageBack(); | |
| await tester.pumpAndSettle(); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/test/userflow_programs_screen_deep_test.dart` around lines 91
- 98, Update the FAB sweep in the test to capture the initial
FloatingActionButton finder results or count before tapping, then iterate over
that stable collection while ensuring each tap still targets the intended
ProgramsScreen FABs rather than widgets introduced by navigation. Keep the
existing tap and pumpAndSettle behavior.
Comment on lines
+155
to
+166
| testWidgets('CreateRoutineScreen supports reordering exercise into final position before Add Exercises', (tester) async { | ||
| await tester.pumpWidget(_buildTestApp( | ||
| workoutProvider: workoutProvider, | ||
| settingsProvider: settingsProvider, | ||
| child: const CreateRoutineScreen(), | ||
| )); | ||
| await tester.pumpAndSettle(); | ||
|
|
||
| final reorderableList = tester.widget<ReorderableListView>(find.byType(ReorderableListView)); | ||
| reorderableList.onReorderItem!(0, 1); | ||
| await tester.pumpAndSettle(); | ||
| expect(tester.takeException(), isNull); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exercise a real final-position reorder.
The screen has no selected exercises, so onReorderItem!(0, 1) returns immediately at routine_creator.dart Line 149. Seed at least two selected exercises, invoke the final destination index, and assert their displayed order changes; otherwise this passes even if the final-position fix regresses.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/test/userflow_routine_creation_test.dart` around lines 155 -
166, Update the test CreateRoutineScreen supports reordering exercise into final
position before Add Exercises to seed at least two selected exercises in the
providers or test state before building the screen. Invoke onReorderItem with a
destination index representing the final position, then assert the displayed
exercise order changes; retain the no-exception assertion so the test exercises
the actual final-position reorder path.
Comment on lines
+96
to
+103
| // Tap FAB to add new target | ||
| final fab = find.byType(FloatingActionButton); | ||
| if (fab.evaluate().isNotEmpty) { | ||
| await tester.tap(fab.first); | ||
| await tester.pumpAndSettle(); | ||
| } | ||
|
|
||
| expect(tester.takeException(), isNull); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require and assert each advertised workflow outcome.
A missing or no-op control currently still passes these coverage tests.
workout-logger/test/userflow_targets_and_muscle_sheets_full_test.dart#L96-L103: require the FAB and assert the add-target dialog appears.workout-logger/test/userflow_targets_and_muscle_sheets_full_test.dart#L138-L154: require the field and buttons; assert phase/week creation and save feedback or persistence.workout-logger/test/screens/programs/programs_screens_test.dart#L25-L29: assert the FAB interaction reaches its expected destination.workout-logger/test/screens/programs/programs_screens_test.dart#L73-L76: assert successful validation feedback after submitting valid JSON.
📍 Affects 2 files
workout-logger/test/userflow_targets_and_muscle_sheets_full_test.dart#L96-L103(this comment)workout-logger/test/userflow_targets_and_muscle_sheets_full_test.dart#L138-L154workout-logger/test/screens/programs/programs_screens_test.dart#L25-L29workout-logger/test/screens/programs/programs_screens_test.dart#L73-L76
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/test/userflow_targets_and_muscle_sheets_full_test.dart` around
lines 96 - 103, Strengthen the workflow tests so advertised outcomes are
required rather than skipped: in
workout-logger/test/userflow_targets_and_muscle_sheets_full_test.dart:96-103,
require the FloatingActionButton and assert the add-target dialog appears; at
:138-154, require the target field and action buttons, then assert phase/week
creation and save feedback or persistence. In
workout-logger/test/screens/programs/programs_screens_test.dart:25-29, assert
the FAB reaches its expected destination, and at :73-76, assert successful
validation feedback after submitting valid JSON.
Comment on lines
+59
to
+66
| await robot.pumpScreen( | ||
| const TargetsTab(), | ||
| storage: storage, | ||
| workoutProvider: workoutProvider, | ||
| settingsProvider: settingsProvider, | ||
| ); | ||
|
|
||
| robot.expectVisible(TargetsTab); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Assert the behaviors named by these tests.
The populated-target test only verifies the screen shell, and the editable-card test never interacts with the card or validates a callback. Assert target-card content after persistence, then trigger an edit and verify captured callback arguments.
Also applies to: 86-115
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/test/userflow_targets_and_muscle_sheets_test.dart` around
lines 59 - 66, Strengthen the tests around TargetsTab: after persisting
populated targets, assert the rendered target-card content rather than only
TargetsTab visibility; in the editable-card test, interact with the card’s edit
control and verify the captured callback arguments. Update the relevant test
cases and their existing callback capture setup, preserving the current
persistence and screen setup.
Comment on lines
+120
to
+127
| await robot.pumpScreen( | ||
| const ReadinessCard(), | ||
| storage: storage, | ||
| workoutProvider: workoutProvider, | ||
| settingsProvider: settingsProvider, | ||
| ); | ||
|
|
||
| expect(tester.takeException(), isNull); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Provide a ready snapshot before claiming ReadinessCard renders.
No readiness manager is injected, so the harness supplies one with no snapshot. ReadinessCard therefore builds its empty state; assert a ready snapshot, visible card content, and the drill-down interaction.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/test/userflow_targets_and_muscle_sheets_test.dart` around
lines 120 - 127, Update the ReadinessCard test setup to inject a readiness
manager containing a ready snapshot, then assert the card’s visible content and
drill-down interaction before checking for exceptions. Use the existing
harness/provider symbols and preserve the no-exception assertion after
validating the rendered state.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit